Skip to content

perf: index graph parents and successors for reply paths - #1417

Merged
xHeaven merged 2 commits into
dev-null-sh:productionfrom
MrPunyapal:perf/index-commit-graph-relations
Nov 6, 2026
Merged

xHeaven merged 2 commits into
dev-null-sh:productionfrom
MrPunyapal:perf/index-commit-graph-relations

Conversation

@MrPunyapal

@MrPunyapal MrPunyapal commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Building reply paths repeatedly searches every measured graph node for a parent’s replies and continuation. This groups those relationships once, so each lookup scans the matching nodes in their original order.

All 33 graph-edge tests passed, along with TypeScript checks and the build. I also compared complete edge results with the original implementation for small graphs, many unrelated threads, and a large sibling group, including drawing-state variants. The results matched.

This removes scans of unrelated nodes. A single large sibling group can still require quadratic filtering; the PR does not claim linear behavior or a measured browser rendering improvement. GitHub’s test workflow is awaiting repository approval.

Summary by CodeRabbit

  • Bug Fixes
    • Improved consistency of thread branch connections when a thread is displayed alongside many unrelated nodes.
  • Tests
    • Added coverage to verify that reply and continuation connections remain consistent as unrelated nodes are added.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 36c7af06-4641-488c-aa11-c7d59a4b16a0
Commits

Reviewing files that changed from the base of the PR and between a72afa3 and 148b293.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

graphEdges groups nodes by parent and predecessor before building branch edges. The branch logic uses these groups to find successors and earlier replies. A regression test checks that adding 200 unrelated nodes does not change the thread’s reply and continuation edge IDs or paths.

Changes: Graph edge lookup

Layer / File(s) Summary
Grouped branch-edge lookup
resources/js/lib/graph.ts, tests/Unit/GraphEdges.test.ts
graphEdges groups nodes by parent and predecessor, then uses those groups to find branch successors and earlier replies. A regression test checks edge IDs and paths with 200 unrelated nodes added.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related issues

Possibly related PRs

Suggested reviewers: jubianchi

Merge Risk: Minimal · previous review a72af

No concrete correctness or availability risk is established by this change.

Architecture risk: Low · up to a72af

The change affects 2 systems.

Changed systems: resources, tests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — resources (service) was modified; 1 changed file maps to changed impact.
  • observed — tests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in resources/js/lib/graph.ts: graphEdges now builds maps of nodes keyed by parent and predecessor (after) before creating edges, replacing the prior edge-array initialization at this point.
  • observed — Modified behavior in resources/js/lib/graph.ts: The branch logic now checks only the parent’s grouped successors for a lower, non-drawing trunk, and selects earlier replies from its grouped replies. These replace scans across all nodes; the filtering conditions are unchanged.
  • observed — Modified behavior in tests/Unit/GraphEdges.test.ts: Adds a regression test that compares the thread’s edge IDs and paths with and without 200 unrelated nodes; it expects the results to be identical.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: indexing graph parents and successors to improve reply-path performance.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 256e64f2-0601-40c3-b8bc-7470d5c9da52
Commits

Reviewing files that changed from the base of the PR and between 2c15b1d and a72afa3.

Files selected for processing: 1

  • tests/Unit/GraphEdges.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@MrPunyapal
MrPunyapal force-pushed the perf/index-commit-graph-relations branch from b58c274 to b8df0f6 Compare November 3, 2026 05:55
@MrPunyapal
MrPunyapal marked this pull request as ready for review November 3, 2026 06:02
@MrPunyapal
MrPunyapal force-pushed the perf/index-commit-graph-relations branch from b8df0f6 to 2c15b1d Compare November 3, 2026 21:04
@MrPunyapal
MrPunyapal force-pushed the perf/index-commit-graph-relations branch from 2c15b1d to a72afa3 Compare November 4, 2026 20:46
@xHeaven
xHeaven merged commit c3a4d6f into dev-null-sh:production Nov 6, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants